Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Platform] Add platform pluggable framework #11222

Closed
wants to merge 2 commits into from
Closed

[Platform] Add platform pluggable framework #11222

wants to merge 2 commits into from

Conversation

wangxiyuan
Copy link
Contributor

@wangxiyuan wangxiyuan commented Dec 16, 2024 •
edited by github-actions bot
Loading

See RFC: #11162

This PR is the first step of the RFC. More PR will be added until reach the RFC goal.

What is Done:

  • Added PlatformRegister class. Provide some APIs for plugin to register and initialize out-of-tree platform
  • Added a CurrentPlatform wrapper to make sure the current_platform global var is always up-to-date.
  • Added an example about how to register a new out-of-tree platform plugin
  • Created a new test pipeline for plugin testing

Some refactor should be done in other PRs, see #11162 (comment)

How to use:

  1. The user implement his platform library, for example, the name called vllm_new_device_plugin.
  2. install vLLM and the plugin package 
    pip install vllm vllm_new_device_plugin
  3. use vLLM the same as before. Then the inference will run on the new device. 
    import vllm
vllm.LLM(xxx)

@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@wangxiyuan wangxiyuan mentioned this pull request Dec 16, 2024
Open
1 task
@wangxiyuan wangxiyuan force-pushed the platform_pluggable branch 2 times, most recently from 6b0a018 to 48a49bd Compare December 16, 2024 02:43
@wangxiyuan wangxiyuan changed the title [POC] Add platform pluggable [POC][Platform] Add platform pluggable Dec 16, 2024
@wangxiyuan wangxiyuan force-pushed the platform_pluggable branch 6 times, most recently from b43b61c to 3ec575e Compare December 16, 2024 03:56
@wangxiyuan

This comment was marked as outdated.

Sign in to view
@wangxiyuan wangxiyuan force-pushed the platform_pluggable branch 2 times, most recently from f31d075 to 1cb1934 Compare December 16, 2024 11:38
@wangxiyuan wangxiyuan force-pushed the platform_pluggable branch 2 times, most recently from 189a199 to a4b6e8c Compare December 17, 2024 12:04
@wangxiyuan

This comment was marked as outdated.

Sign in to view
@mergify mergify bot added the documentation Improvements or additions to documentation label Dec 18, 2024
@wangxiyuan wangxiyuan changed the title [POC][Platform] Add platform pluggable [Platform] Add platform pluggable - 1 Dec 18, 2024
@wangxiyuan wangxiyuan changed the title [Platform] Add platform pluggable - 1 [Platform] Add platform pluggable framework Dec 18, 2024
@wangxiyuan wangxiyuan force-pushed the platform_pluggable branch 6 times, most recently from fbf37c1 to 75eedc0 Compare December 23, 2024 06:24
@wangxiyuan
Copy link
Contributor Author

This PR is ready for review now. @DarkLight1337 @simon-mo @youkaichao

DarkLight1337
DarkLight1337 reviewed Dec 24, 2024
View reviewed changes
vllm/platforms/__init__.py Outdated Show resolved Hide resolved
tests/plugins/vllm_add_dummy_platform/setup.py Show resolved Hide resolved
vllm/platforms/__init__.py Outdated
Comment on lines 34 to 44
def __getattr__(self, name):
"""Get the attribute. If the attribute is not found, go pass to the
current platform."""
if name == 'platform':
return self.platform
if name == 'initialize_current_platform':
return self.initialize_current_platform
# Go pass to the current platform.
return getattr(self.platform, name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To enable IDE autocompletion for the wrapped platform's methods, we might have to use TYPE_CHECKING flag to provide more detailed type information separate from the real declaration of the class.

Copy link
Contributor Author

@wangxiyuan wangxiyuan Dec 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to do statical type check to infer the reference of dynamical attributes returned by getattr.

I did a new way to implement the wrapper. CurrentPlatform inherits from Platform interface now. In this way, IDE autocompletion works.

@mergify mergify bot added the ci/build label Dec 24, 2024
@wangxiyuan wangxiyuan force-pushed the platform_pluggable branch 2 times, most recently from 3b404bc to f524313 Compare December 24, 2024 09:20
DarkLight1337
DarkLight1337 reviewed Dec 24, 2024
View reviewed changes
.buildkite/test-pipeline.yaml Outdated
Comment on lines 472 to 473
- pip install -e ./plugins/vllm_add_dummy_model
- pytest -v -s distributed/test_distributed_oot.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we probably can't move this because it needs distributed environment...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can require 2 GPUs for the full Plugin Test pipeline.

Copy link
Contributor Author

@wangxiyuan wangxiyuan Dec 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my mistake. Will update later to avoid wasting CI resource. Considered the plugin test is simple currently, let's do not waste CI resource. We can do it once plugin test is rich. I'll leave a note then

@DarkLight1337
Copy link
Member

DarkLight1337 commented Dec 24, 2024 •
edited
Loading

@youkaichao please also verify this when you are free.

@wangxiyuan wangxiyuan requested a review from ywang96 as a code owner December 24, 2024 12:11
@wangxiyuan
Add platform pluggable
fc4da42 
Signed-off-by: wangxiyuan <[email protected]>
@wangxiyuan
Update plugin test
dbd0113 
Signed-off-by: wangxiyuan <[email protected]>
@youkaichao youkaichao self-assigned this Dec 28, 2024
@youkaichao youkaichao mentioned this pull request Dec 29, 2024
Merged
@youkaichao
Copy link
Member

thanks for the contribution! However, I think model runner is not mature enough to be exposed right now.

I added #11602 to enable platform plugins, please help me review that pr, thanks! @wangxiyuan @MengqingCao

@wangxiyuan
Copy link
Contributor Author

See #11602

@wangxiyuan wangxiyuan closed this Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MengqingCao MengqingCao MengqingCao left review comments

@DarkLight1337 DarkLight1337 DarkLight1337 left review comments

@robertgshaw2-neuralmagic robertgshaw2-neuralmagic Awaiting requested review from robertgshaw2-neuralmagic robertgshaw2-neuralmagic is a code owner

@simon-mo simon-mo Awaiting requested review from simon-mo simon-mo is a code owner

@youkaichao youkaichao Awaiting requested review from youkaichao

@ywang96 ywang96 Awaiting requested review from ywang96 ywang96 is a code owner

Assignees

@youkaichao youkaichao

Labels
ci/build documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wangxiyuan @MengqingCao @DarkLight1337 @youkaichao